test: Parallelize the End to End tests - Part two enablement#2033
Conversation
Signed-off-by: Shmuel Kallner <kallner@il.ibm.com>
Signed-off-by: Shmuel Kallner <kallner@il.ibm.com>
Signed-off-by: Shmuel Kallner <kallner@il.ibm.com>
Signed-off-by: Shmuel Kallner <kallner@il.ibm.com>
Signed-off-by: Shmuel Kallner <kallner@il.ibm.com>
Signed-off-by: Shmuel Kallner <kallner@il.ibm.com>
Signed-off-by: Shmuel Kallner <kallner@il.ibm.com>
Signed-off-by: Shmuel Kallner <kallner@il.ibm.com>
Signed-off-by: Shmuel Kallner <kallner@il.ibm.com>
Signed-off-by: Shmuel Kallner <kallner@il.ibm.com>
Signed-off-by: Shmuel Kallner <kallner@il.ibm.com>
Signed-off-by: Shmuel Kallner <kallner@il.ibm.com>
Signed-off-by: Shmuel Kallner <kallner@il.ibm.com>
Signed-off-by: Shmuel Kallner <kallner@il.ibm.com>
Signed-off-by: Shmuel Kallner <kallner@il.ibm.com>
Signed-off-by: Shmuel Kallner <kallner@il.ibm.com>
Signed-off-by: Shmuel Kallner <kallner@il.ibm.com>
Signed-off-by: Shmuel Kallner <kallner@il.ibm.com>
Signed-off-by: Shmuel Kallner <kallner@il.ibm.com>
Signed-off-by: Shmuel Kallner <kallner@il.ibm.com>
Signed-off-by: Shmuel Kallner <kallner@il.ibm.com>
Signed-off-by: Shmuel Kallner <kallner@il.ibm.com>
Signed-off-by: Shmuel Kallner <kallner@il.ibm.com>
Signed-off-by: Shmuel Kallner <kallner@il.ibm.com>
|
lint not passing |
elevran
left a comment
There was a problem hiding this comment.
Parallelizing the e2e suite via Ginkgo's multi-process runner is a good direction and the 10m saving is significant - thanks!
Two things need attention before merge:
- CI lint is currently failing (staticcheck QF1001 in setup_test.go) - see inline comment.
- The new NodePort computation ignores any overrides in
E2E_PORT/E2E_METRICS_PORTand would no longer match the actual NodePort of the created Service.
|
|
||
| ginkgo.AfterAll(func() { | ||
| // Only cleanup if the test succeeded | ||
| if !(ginkgo.CurrentSpecReport().Failed() && keepClusterOnFailure) { |
There was a problem hiding this comment.
CI lint fails
| if !(ginkgo.CurrentSpecReport().Failed() && keepClusterOnFailure) { | |
| if !ginkgo.CurrentSpecReport().Failed() || !keepClusterOnFailure { |
|
|
||
| infraSubs := map[string]string{ | ||
| "${EPP_NAME}": "e2e-epp", | ||
| "${METRICS_NODE_PORT}": strconv.Itoa(32090 + 100*(ginkgo.GinkgoParallelProcess()-1)), |
There was a problem hiding this comment.
This hardcodes 32090 instead of using baseMetricsPort, diverging from getMetricsPort() (via testutils.ProcessPort(baseMetricsPort)) used elsewhere in this file.
If a user overrides E2E_METRICS_PORT, the client dials the overridden port but the NodePort Service created here still binds to 32090-based values.
| func createEnvoy(nsName string) ([]string, *gexec.Session) { | ||
| infraSubs := map[string]string{ | ||
| "${NAMESPACE}": nsName, | ||
| "${ENVOY_NODE_PORT}": strconv.Itoa(30080 + 100*(ginkgo.GinkgoParallelProcess()-1)), |
There was a problem hiding this comment.
Same issue as the metrics port: this hardcodes 30080 instead of basePort.
Overriding E2E_PORT would break the NodePort/client-port match.
|
|
||
| // testWrapper wraps tests with the setup and teardown code needed. | ||
| // It is used as a wrapper of the function passed to ginkgo.When calls that | ||
| // setup the tests. It is important that the gink.Ordered decorator is used |
There was a problem hiding this comment.
| // setup the tests. It is important that the gink.Ordered decorator is used | |
| // setup the tests. It is important that the ginkgo.Ordered decorator is used |
| E2E_NUM_PROCS=1 go test -v -timeout 45m "${pkg}" -ginkgo.v -ginkgo.fail-fast "-ginkgo.label-filter=${E2E_LABEL_FILTER}" | ||
| else | ||
| go test -v -timeout 45m "${pkg}" -ginkgo.v -ginkgo.fail-fast | ||
| ginkgo run --procs=${E2E_NUM_PROCS} --timeout 45m -v --fail-fast "${pkg}" |
There was a problem hiding this comment.
nit:
${E2E_NUM_PROCS} is unquoted here, unlike the other variable expansions in this function (e.g., "${pkg}", "${E2E_LABEL_FILTER}"). Low risk but worth being consistent.
Signed-off-by: Shmuel Kallner <kallner@il.ibm.com>
Signed-off-by: Shmuel Kallner <kallner@il.ibm.com>
Signed-off-by: Shmuel Kallner <kallner@il.ibm.com>
|
@elevran Thanks for the thorough review. I believe all of theissues raised have been fixed. |
What type of PR is this?
/kind test
What this PR does / why we need it:
Currently the llm-d-router End to End tests run for quite a long time (about sixteen minutes on my Mac M4 Max).
This long time has affected the CI pipeline in the past. A work around was done to run the tests in parts in parallel in separate Kind clusters. This splitting up of the tests needs to be managed manually and has overhead as prepetory steps are done over and over again for each test "slice'
A better solution is to run tests in parallel on the same Kind cluster using K8S Namespaces for separation as needed.
This PR enables the End to End tests to run in parallel. The tests are broken up into groups, mostly by Ginkgo When blocks. By default five such groups run in parallel, all on the same Kind cluster, each using its own namespace with an envoy instance and its own nodePort.
On my laptop a Mac M4 (Max) this PR has reduced the time to run the tests from about sixteen minutes, to between five and six minutes, a significant improvement.
A future PR will update CI to exploit this PR and remove the "test labels"
Which issue(s) this PR fixes:
Refs #1931
Release note (write
NONEif no user-facing change):